Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add MIMA checks to ensure binary compatibility across different versions #1018

Closed
wants to merge 3 commits into from

Conversation

myazinn
Copy link
Contributor

@myazinn myazinn commented Aug 9, 2023

No description provided.

@myazinn myazinn requested a review from erikvanoosten as a code owner August 9, 2023 22:02
@CLAassistant
Copy link

CLAassistant commented Aug 9, 2023

CLA assistant check
All committers have signed the CLA.


lazy val lint = {
val defaultLint = zio.sbt.Commands.ComposableCommand.lint
defaultLint.copy(commandStrings = defaultLint.commandStrings :+ "mimaCheck").toCommand
Copy link
Contributor Author

@myazinn myazinn Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an awful hack that I'm not proud of. But it's the only one that I can think of that works ¯\_(ツ)_/¯

build.sbt Show resolved Hide resolved
@myazinn myazinn force-pushed the add-sbt-mima-plugin branch from f89d51a to c8f242b Compare August 9, 2023 22:09
@erikvanoosten
Copy link
Collaborator

I propose that (for now) we only keep patch versions binary compatible. So minor version may break it.
As we gain confidence (and/or find the need) we may extend this to keep minor versions binary compatible.

@myazinn is there a way to automate this? Right now I see that Mima will always compare to 2.4.2.

@erikvanoosten
Copy link
Collaborator

🚀 @myazinn Thanks for your contribution!

@myazinn
Copy link
Contributor Author

myazinn commented Aug 16, 2023

Hi @erikvanoosten :) Apologies for not replying for so long.

@myazinn is there a way to automate this? Right now I see that Mima will always compare to 2.4.2.

Yes, but only up to a point. We can always use the latest version to compare like this

lazy val bincompatVersionToCompare = "latest.integration"

Or choose the latest patch like this (maybe there's an easier way though).

  def mimaSettings(failOnProblem: Boolean) =
    Seq(
      mimaPreviousArtifacts := {
        version.value match {
          case VersionNumber(Seq(major, minor, patch), _, _) =>
            Set(organization.value %% name.value % s"$major.$minor.$patch")
          case _ =>
            Set.empty
        }
      },
      mimaBinaryIssueFilters ++= Seq(
        exclude[Problem]("zio.kafka.consumer.internal.*")
      ),
      mimaFailOnProblem := failOnProblem
    )

Or using ivy wildcards.

But

propose that (for now) we only keep patch versions binary compatible.

This is a tricky part. When you make a MR, you don't actually know which next version will be released, so the checker can't decide whether to fail the build or not. It should be possible to do something like this

  def mimaSettings(failOnProblem: Boolean) =
    Seq(
      mimaPreviousArtifacts := {
        version.value match {
          // patch == 0 means there's a new minor or major version, which doesn't have to maintain binary compatibility
          case VersionNumber(Seq(_, _, 0), _, _) =>
            Set.empty
          case VersionNumber(Seq(major, minor, patch), _, _) =>
            Set(organization.value %% name.value % s"$major.$minor.+")
          case _ =>
            Set.empty
        }
      },
      mimaBinaryIssueFilters ++= Seq(
        exclude[Problem]("zio.kafka.consumer.internal.*")
      ),
      mimaFailOnProblem := failOnProblem
    )

and run this when the new version was cut and is being build. It's a safer option, but far less convenient, since the release might be blocked because of some compatibility issues caused by MR merged weeks ago.

erikvanoosten added a commit that referenced this pull request Apr 14, 2024
Add MIMA checks to ensure binary compatibility across different
zio-kafka versions.

Also: use scala 3 syntax in sbt files.

Based on #1018 by @myazinn.

Mima is disabled in this PR because master is currently _not_ binary
compatible with the latest released version.

---------

Co-authored-by: Nikita Miazin <[email protected]>
@erikvanoosten
Copy link
Collaborator

Replaced by #1214.

@erikvanoosten
Copy link
Collaborator

Thanks @myazinn !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants